Add support for Workflow resource by naclonts · Pull Request #2 · aws-controllers-k8s/mwaaserverless-controller

naclonts · 2026-07-13T22:19:18Z

Issue: aws-controllers-k8s/community#2761

Description: Adds the Workflow resource to the MWAA Serverless controller.

This adds:

Workflow CRD with CRUD
ARN primary key, GetWorkflow-based read path
Tag sync via ListTagsForResource + TagResource/UntagResource (MWAA Serverless has no inline Tags member on Create/Get/Update; UpdateWorkflow cannot manage tags)
Cross-resource references: iam Role, s3 Bucket, kms Key, ec2 SecurityGroup and Subnet
Print columns, synced predicate, EngineVersion handling, and exception / terminal error-code mapping
E2E tests covering create → READY, update, delete, and terminal-condition handling

Tested against the MWAA Serverless API.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Implements the Workflow CRD (full CRUD) for the MWAA Serverless controller. - ARN primary key with GetWorkflow-based read path - Tag sync via ListTagsForResource + Tag/UntagResource hooks (UpdateWorkflow has no inline Tags member), implemented in pkg/tags and wired through the workflow read/update hooks - Cross-resource references: iam Role, s3 Bucket, kms Key, ec2 SecurityGroup and Subnet - Print columns, synced predicate, EngineVersion handling, and exception / terminal error code mapping Two fixes from live end-to-end testing against the MWAA Serverless API: - delete targets the whole workflow, not just a single version (sdk_delete_post_build_request) - read path preserves EngineVersion, TriggerMode, and LoggingConfiguration to stop phantom UpdateWorkflow churn (sdk_read_one_post_set_output / sdk_update_pre_build_request) Builds on the bootstrap PR (adds the Workflow resource that the bootstrap placed in ignore.resource_names).

ack-prow · 2026-07-16T18:35:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: naclonts
Once this PR has been reviewed and has the lgtm label, please assign michaelhtm for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…d e2e fixtures

naclonts · 2026-07-17T23:36:50Z

/retest

GetWorkflow never returns EngineVersion, so the generated read path fabricates Spec.EngineVersion=0 (int32 zero value) on every read. When a user omits spec.engineVersion, the old nil-guarded read hook skipped restoring the desired value and late_initialize persisted the fabricated 0 into the CR spec. A subsequent UpdateWorkflow then sent EngineVersion=0, which the API rejects with `ValidationException: Invalid engine version` (only value 1 is valid). The existing e2e masked this because its fixture hardcodes engineVersion: 1. Fix: - generator.yaml: replace `late_initialize: {}` with `compare: {is_ignored: true}` on EngineVersion. Removes the late-init backfill (and its requeue-until-set livelock risk) and drops the field from delta comparison, since GetWorkflow provides no observable ground truth to diff. - sdk_read_one_post_set_output hook: mirror desired onto observed unconditionally (including nil) so an unset field stays absent and a set field round-trips, leaving no phantom EngineVersion delta. - e2e: add workflow-no-engine-version.yaml and an unset-engineVersion lifecycle (run sequentially within the single test function to preserve the one-workflow-in-flight xdist invariant) asserting spec.engineVersion stays nil and workflowVersion is stable across a resync. Regenerated with `make build-controller` (code-generator 01fb153, aws-sdk-go v1.42.1). The create/update payloads still send EngineVersion when the user sets it.

knottnt

Thanks @naclonts. Left some comments.

knottnt · 2026-07-20T23:45:34Z

-  resource_names:
-    - Workflow
+  field_paths:
+    - CreateWorkflowOutput.Warnings


Q: what's the reason for ignoring this field?

Looks like this is emitted during workflow creation, if the create succeeded but had some warnings. Not sure if it'd make sense to surface this anywhere, since we'd get a seperate SDK error if the creation actually fails

knottnt · 2026-07-20T23:51:14Z

+        from:
+          operation: GetWorkflow
+          path: ScheduleConfiguration
+      WorkflowDefinition:


Q: How large can this definition get? We may want to consider omitting this if use can create arbitrarily large definitions to avoid breaching ectd limits.

Presumably, the user will be managing this definition outside of this YAML file anyway.

Makes sense, added to ignore

knottnt · 2026-07-20T23:56:55Z

+CREATE_TIMEOUT_SECONDS = 60 * 20
+UPDATE_TIMEOUT_SECONDS = 60 * 20
+DELETE_TIMEOUT_SECONDS = 60 * 15


Just to double check are these wait times needed?

Nope, run time is much shorter (~4 minutes). Simplified these to one TIMEOUT_SECONDS

knottnt · 2026-07-21T00:19:51Z

+		ko.Spec.Tags = tags
+	}
+
+	// Preserve the user's desired values for fields that GetWorkflow reports


Just a meta comment on this approach. If we need custom code for handling how delta.go detects comparisons for fields it is generally better to handle that with hooks like delta_pre_compare or delta_post_compare. It's somewhat unexpected for sdkFind to concern itself with the comparison of a field with desired state as it should be primarily concerned with reading the current state in AWS.

Makes sense. Updated to just normalize EngineVersion=0 to the desired value.

…ld renames - late_initialize TriggerMode/LoggingConfiguration (GetWorkflow echoes server defaults) instead of masking phantom deltas in sdkFind - detect EngineVersion drift via the generated comparator; keep only a narrow read-time zero-normalization so unset stays unset and future values update - rename CRD fields via go_tag: type_->type, workflowStatus->status, workflowVersion->version - ignore GetWorkflowOutput.WorkflowDefinition to keep the CR small in etcd - trim verbose fixture/hook comments

verify-code-gen regenerates release artifacts too; the helm chart CRD had stale field names (workflowStatus/workflowVersion/type_) and the removed workflowDefinition status field. Regenerated via build-controller-release.sh so it matches config/crd/bases.

…tionale - replace CREATE/UPDATE/DELETE_TIMEOUT_SECONDS (20m/20m/15m) with a single TIMEOUT_SECONDS=8m; the full lifecycle runs in ~4m in e2e - reword the single-test-function docstring: unique names mean one-workflow-at- a-time is throttling/quota/teardown hygiene, not a correctness requirement - drop a redundant tag comment from the sdk_read_one_post_set_output hook

Unique per-test names make the workflows independent, so there's no reason to serialize them into one test function. Promote the unset-engineVersion case to its own test_workflow_unset_engine_version and drop the single-function / xdist-serialization rationale.

ack-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 13, 2026

naclonts changed the title ~~feat: add support for Workflow resource~~ Add support for Workflow resource Jul 13, 2026

naclonts force-pushed the add-workflow-resource branch from 6edf817 to dc9ab1e Compare July 16, 2026 18:35

naclonts marked this pull request as ready for review July 16, 2026 18:37

ack-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 16, 2026

ack-prow Bot requested review from gustavodiaz7722 and sapphirew July 16, 2026 18:37

Nathan Clonts added 3 commits July 16, 2026 12:56

fix: value-aware drift detection, non-terminal quota errors, and vali…

1f26f23

…d e2e fixtures

chore: regenerate metadata (runtime v0.61.0) and ATTRIBUTION.md

83ddc98

test: assert workflowVersion is stable across a no-op resync

6f65f5c

knottnt reviewed Jul 21, 2026

View reviewed changes

Nathan Clonts added 4 commits July 21, 2026 13:34

naclonts requested a review from knottnt July 21, 2026 21:54

Uh oh!

Conversation

naclonts commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ack-prow Bot commented Jul 16, 2026

Uh oh!

naclonts commented Jul 17, 2026

Uh oh!

knottnt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

naclonts Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

naclonts commented Jul 13, 2026 •

edited

Loading

naclonts Jul 21, 2026 •

edited

Loading